Skip to content

feat: implement ingress primitive#21

Merged
sourcehawk merged 26 commits intomainfrom
feature/ingress-primitive
Mar 25, 2026
Merged

feat: implement ingress primitive#21
sourcehawk merged 26 commits intomainfrom
feature/ingress-primitive

Conversation

@sourcehawk
Copy link
Owner

Implements the ingress Kubernetes resource primitive following the pattern established by the existing ConfigMap and Deployment primitives.

Summary

  • Adds ingress primitive package under pkg/primitives/ingress/
  • Implements required lifecycle interfaces
  • Includes editors, mutator, flavors, and builder

Checklist

  • Compiles cleanly
  • Tests pass
  • Follows naming conventions in CONTEXT.md
  • Does not modify shared files

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Kubernetes Ingress primitive to the operator component framework, following the existing primitive patterns (builder/resource + mutator pipeline + flavors + lifecycle handlers), and includes an accompanying example and documentation.

Changes:

  • Introduces pkg/primitives/ingress (builder/resource, mutator, handlers, flavors) plus unit tests.
  • Adds a shared IngressSpecEditor under pkg/mutation/editors (with tests) to support typed ingress spec mutations.
  • Adds an examples/ingress-primitive walkthrough and new docs/primitives/ingress.md documentation.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/primitives/ingress/resource.go Defines the ingress Resource wrapper over generic.IntegrationResource.
pkg/primitives/ingress/builder.go Builder API wiring for applicators, flavors, mutations, status/suspension handlers, data extraction.
pkg/primitives/ingress/mutator.go Plan/apply mutator with per-feature planning and category ordering.
pkg/primitives/ingress/handlers.go Default operational/suspension handler implementations for integration lifecycle.
pkg/primitives/ingress/flavors.go Ingress flavors (label/annotation preservation) via shared pkg/flavors.
pkg/primitives/ingress/*_test.go Unit tests for ingress builder/mutator/handlers/flavors behavior.
pkg/mutation/editors/ingressspec.go Adds typed IngressSpecEditor for ingress spec mutations.
pkg/mutation/editors/ingressspec_test.go Tests for IngressSpecEditor operations (class name, backend, rules, TLS).
examples/ingress-primitive/main.go Standalone example driving reconciliation steps using a fake client.
examples/ingress-primitive/resources/ingress.go Example ingress resource factory using mutations, flavors, and data extraction.
examples/ingress-primitive/features/mutations.go Example feature-gated ingress mutations (version annotation + TLS).
examples/ingress-primitive/app/controller.go Example controller that builds a component around the ingress primitive.
examples/ingress-primitive/app/owner.go Re-exports shared ExampleApp types for the example package.
examples/ingress-primitive/README.md Documents how to run and understand the ingress example.
docs/primitives/ingress.md New primitive documentation covering usage, ordering, handlers, flavors, guidance.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.

@sourcehawk
Copy link
Owner Author

Claude Review Cycle 1 Complete

Addressed:

  • Doc comment in resource.go: Fixed interface references from component.* to concepts.Operational, concepts.Suspendable, concepts.DataExtractable (kept component.Resource as that is the correct package)
  • Resource wrapper tests: Added resource_test.go with tests for Identity, Object deep-copy, Mutate, WithMutation, FeatureOrdering, CustomFieldApplicator, ConvergingStatus, DeleteOnSuspend, Suspend, SuspensionStatus, and ExtractData
  • Operational status names in docs: Changed Pending to OperationPending in the capabilities table and status handler table to match concepts.OperationalStatus constants
  • Link from primitives.md: Added ingress to the built-in primitives table and added IngressSpecEditor to the editors table
  • Cross-reference in ingress.md: Added link back to primitives overview page

Intentionally ignored:

  • PR description checklist (ingressspec.go comment): This is about the PR description metadata, not a code change. The shared editor is intentional and minimal.
  • FeatureMutator interface (mutator.go comment): The unexported beginFeature() pattern is identical across all primitives (configmap, deployment, ingress). This is a pre-existing architectural pattern in the framework, not an ingress-specific issue. Fixing it would require changes to internal/generic shared infrastructure, which is out of scope for this PR.

<!-- claude-review-cycle -->

Copilot AI review requested due to automatic review settings March 22, 2026 19:50
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

@sourcehawk
Copy link
Owner Author

approved

@sourcehawk sourcehawk force-pushed the feature/ingress-primitive branch from c21b61c to c31fd20 Compare March 23, 2026 00:13
Copilot AI review requested due to automatic review settings March 23, 2026 03:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

@sourcehawk
Copy link
Owner Author

Claude Review Cycle 1 Complete

Addressed:

  • Status preservation in DefaultFieldApplicator (resource.go:17): Save and restore the live Status field before overwriting with desired.DeepCopy() so that ingress-controller-owned fields like Status.LoadBalancer.Ingress are not cleared during reconciliation.
  • Status preservation test coverage (resource_test.go:191): Added TestDefaultFieldApplicator_PreservesStatus to verify Status.LoadBalancer.Ingress entries survive the field applicator. Added TestDefaultOperationalStatusHandler_Operational and TestDefaultOperationalStatusHandler_OperationalWithHostname to verify the handler returns Operational when an IP or hostname is assigned.

Intentionally ignored:

  • PR description/checklist accuracy (docs/primitives.md:135): This comment is about PR metadata (the description checklist), not a code issue. The PR description can be updated by the PR author separately.

<!-- claude-review-cycle -->

Copilot AI review requested due to automatic review settings March 23, 2026 16:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

sourcehawk and others added 11 commits March 23, 2026 20:17
Provides SetIngressClassName, SetDefaultBackend, EnsureRule (upsert by
host), RemoveRule, EnsureTLS (upsert by first host), RemoveTLS, and Raw
escape hatch.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements builder, resource, mutator, handlers, and flavors for the
networking.k8s.io/v1 Ingress kind.

Key design decisions:
- DefaultDeleteOnSuspend = false (avoids ingress controller churn)
- DefaultSuspendMutation = no-op (backend 502/503 is correct behaviour)
- Operational status: Pending until load balancer address assigned
- Suspension status: immediately Suspended with reason message

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers operational status, suspension strategy, mutation pipeline,
editors, flavors, and guidance for common use cases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Demonstrates base construction, feature mutations (version annotation,
TLS toggle), field flavors, and data extraction using the ingress
builder and mutator.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address Copilot review: DefaultOperationalStatusHandler now checks
that at least one LoadBalancer.Ingress entry has a non-empty IP or
Hostname, rather than only checking slice length. This prevents
marking an Ingress as Operational when entries exist but have no
assigned address.

Update docs and add test for the empty-entry edge case.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix doc comments to reference concepts.Operational/Suspendable/DataExtractable
  instead of component.* for lifecycle interfaces
- Use correct OperationPending status name in docs and code comments
- Add ingress entry to built-in primitives table in docs/primitives.md
- Add IngressSpecEditor to mutation editors table in docs/primitives.md
- Add cross-reference link to primitives overview in ingress doc
- Add resource_test.go with tests for Identity, Object deep-copy, Mutate,
  mutations, feature ordering, custom applicator, converging status,
  suspension, and data extraction

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix doc comments to reference concepts.Operational/Suspendable/DataExtractable
  instead of component.* for lifecycle interfaces
- Use correct OperationPending status name in docs and code comments
- Add ingress entry to built-in primitives table in docs/primitives.md
- Add IngressSpecEditor to mutation editors table in docs/primitives.md
- Add cross-reference link to primitives overview in ingress doc
- Add resource_test.go with tests for Identity, Object deep-copy, Mutate,
  mutations, feature ordering, custom applicator, converging status,
  suspension, and data extraction

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The DefaultFieldApplicator was replacing the entire object with the
desired state, which wiped ingress-controller-owned fields like
Status.LoadBalancer.Ingress. This caused DefaultOperationalStatusHandler
to never see assigned addresses, keeping the resource permanently pending.

Save and restore the live Status before overwriting with the desired spec
so that readiness detection works correctly.

Also adds test coverage for:
- Status.LoadBalancer preservation through DefaultFieldApplicator
- DefaultOperationalStatusHandler returning Operational with IP
- DefaultOperationalStatusHandler returning Operational with Hostname

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sourcehawk sourcehawk requested review from Copilot and removed request for Copilot March 25, 2026 16:07
@sourcehawk sourcehawk requested review from Copilot and removed request for Copilot March 25, 2026 16:09
@sourcehawk sourcehawk requested a review from Copilot March 25, 2026 16:10
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

Comment on lines +57 to +62
func (m *Mutator) EditObjectMetadata(edit func(*editors.ObjectMetaEditor) error) {
if edit == nil {
return
}
m.active.metadataEdits = append(m.active.metadataEdits, edit)
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EditObjectMetadata (and similarly EditIngressSpec) will panic with a nil-pointer dereference if BeginFeature() hasn’t been called, because m.active is nil. Since these methods don’t return an error, consider making them safe by lazily calling BeginFeature() when m.active == nil (or initializing a default feature plan in NewMutator) so accidental misuse can’t crash reconciliation.

Copilot uses AI. Check for mistakes.
Comment on lines +121 to +128
func (b *Builder) WithDataExtractor(extractor func(networkingv1.Ingress) error) *Builder {
if extractor != nil {
b.base.WithDataExtractor(func(ing *networkingv1.Ingress) error {
return extractor(*ing)
})
}
return b
}
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says the extractor receives a “value copy”, but extractor(*ing) is only a shallow struct copy (maps/slices/pointers still alias the same underlying data). If the intent is to prevent extractors from mutating the reconciled object (even accidentally), pass a deep copy (e.g., call ing.DeepCopy() before dereferencing) so nested fields can’t be mutated through shared references.

Copilot uses AI. Check for mistakes.
networkingv1 "k8s.io/api/networking/v1"
)

// IngressSpecEditor provides a typed API for mutating a Kubernetes IngressSpec.
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description checklist states “Does not modify shared files”, but this PR adds/changes shared framework code under pkg/mutation/editors/ (and also updates shared docs like docs/primitives.md). Either update the PR description/checklist to reflect that shared files are modified, or split the editor/doc changes into their own PR if “no shared file modifications” is a hard requirement.

Copilot uses AI. Check for mistakes.

| Capability | Detail |
| ---------------------- | ---------------------------------------------------------------------------------------------- |
| **Operational status** | Reports `OperationPending` until the ingress controller assigns an address, then `Operational` |
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs reference OperationPending, but the code returns concepts.OperationalStatusPending and the status concept elsewhere is “Pending” vs “Operational”. To avoid confusing API users, align terminology with the actual status names used by the framework (e.g., “Pending”/“Operational”, or explicitly OperationalStatusPending/OperationalStatusOperational).

Suggested change
| **Operational status** | Reports `OperationPending` until the ingress controller assigns an address, then `Operational` |
| **Operational status** | Reports `Pending` until the ingress controller assigns an address, then `Operational` |

Copilot uses AI. Check for mistakes.
@sourcehawk sourcehawk requested a review from Copilot March 25, 2026 16:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

if edit == nil {
return
}
m.active.ingressSpecEdits = append(m.active.ingressSpecEdits, edit)
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EditObjectMetadata/EditIngressSpec will panic if BeginFeature() was not called (m.active is nil). Since Mutator is a public helper, it should fail safely. Consider implicitly calling BeginFeature() when m.active is nil, or changing these methods to return an error when no active feature plan exists (and have callers handle it).

Copilot uses AI. Check for mistakes.
| Condition | Status | Reason |
| ----------------------------------------- | ------------------ | ----------------------------------------- |
| Entry with `IP != ""` or `Hostname != ""` | `Operational` | Ingress has been assigned an address |
| Otherwise | `OperationPending` | Awaiting load balancer address assignment |
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs refer to OperationPending, but the implementation (and other docs/tests) use concepts.OperationalStatusPending / 'Pending' terminology. Please align the documentation to the actual status enum names to avoid confusion for API consumers.

Copilot uses AI. Check for mistakes.
Both methods now call BeginFeature() when no active feature plan exists,
preventing nil-pointer panics when callers omit BeginFeature(). This
preserves the ObjectMutator interface contract (no error return) while
making the Mutator safe for public use without requiring strict call ordering.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sourcehawk
Copy link
Owner Author

Claude Review Cycle 1 Complete

Addressed:

  • mutator.go:76 — EditObjectMetadata and EditIngressSpec now implicitly call BeginFeature() when no active feature plan exists, preventing nil-pointer panics. This approach preserves the ObjectMutator interface contract (no error return) while making the Mutator safe for public use. Also reverted the prior incomplete fix that changed EditObjectMetadata's signature and broke the interface.

Intentionally ignored:

  • ingress.md:226 — The doc uses OperationPending which is the actual string value of concepts.OperationalStatusPending (defined as OperationalStatus = "OperationPending"). The doc correctly shows the status string that API consumers will observe, so no change is needed.

<!-- claude-review-cycle -->

@sourcehawk sourcehawk requested review from Copilot and removed request for Copilot March 25, 2026 16:17
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

@sourcehawk sourcehawk merged commit fac88c0 into main Mar 25, 2026
1 of 2 checks passed
@sourcehawk sourcehawk deleted the feature/ingress-primitive branch March 25, 2026 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants